I'm not sure I understand how we're getting to a state where we have None values arriving here. Do we have an example of what that case might look like?

The rust SDK has experienced issues (aws-sdk-rust#1095) when parsing responses from services like quicksight who populate additional union members with null values. Although the quicksight case doesn't specifically affect boto3 due to differences in modeled shape between Smithy and C2J, it’s possible for any other service to send us this type of response for a union structure.

Related Protocol Test Id: AwsJson10DeserializeAllowNulls

Is there something unique about this case or should this be handled farther down in _parse_xml_string_to_dom? I'm curious what valid cases we're expecting the service to send us a body and we're getting nothing.

Is there something unique about this case or should this be handled farther down in _parse_xml_string_to_dom?

The RestXMLParser handles this similarly by checking for an empty string before passing it to _parse_xml_string_to_dom:

    def _initial_body_parse(self, xml_string):
        if not xml_string:
            return ETree.Element('')
        return self._parse_xml_string_to_dom(xml_string)

It would make sense to move this logic into _parse_xml_string_to_dom to handle both the restxml and query cases.

However, you bring up a good question:

I'm curious what valid cases we're expecting the service to send us a body and we're getting nothing.

The case the protocol tests are handling is if a service doesn't define output members, it expects nothing in the body. REST-XML services send us an empty body, however, query services send us something similar to the following as opposed to an empty body:

<?xml version="1.0"?>
<DeleteDomainResponse xmlns="http://sdb.amazonaws.com/doc/2009-04-15/">
    <ResponseMetadata>
        <RequestId>c192b7ed-2f6f-ad6b-1a38-da6dce8d7bdb</RequestId>
        <BoxUsage>0.0055590278</BoxUsage>
   </ResponseMetadata>
</DeleteDomainResponse>

Conclusion
For now, I think I can move this logic into the test runner instead of our parser until we have known cases where this happens or get more guidance from smithy if this case should actually be handled.

This is semantically different. Are we enforcing that the code is everything following the first #?

If a : character is present, then take only the contents before the first : character in the value.
If a # character is present, then take only the contents after the first # character in the value.

I changed this to try and follow the smithy guidance as close as possible. I'm not aware of any cases where there are multiple # in the protocol tests or services. However, to prevent any unwanted changes in behavior, I'll revert this back to our existing behavior.

There are now 3 distinct if statements here that aren't using elif/else. Is the intent to continually mutate the code value as it goes through each check?

e.g.

com.test.mypackage#this:that#1 -- split(':', 1) --> com.test.mypackage#this
com.test.mypackage# -- split('#', 1) --> this

Then this is evaluated for the x-amzn-query-error code?

Yup that's correct. Smithy provides the following examples:

All of the following error values resolve to FooError:

FooError
FooError:http://internal.amazon.com/coral/com.amazon.coral.validate/
aws.protocoltests.restjson#FooError
aws.protocoltests.restjson#FooError:http://internal.amazon.com/coral/com.amazon.coral.validate/

It looks like we're changing behavior again here. Is there additional scope, or why are we post-processing the body for the code case now?

Do we use PROTOCOL_TEST_IGNORE_LIST_PATH somewhere else? Should this whole thing be a self-contained function or pytest fixture instead?

I'll add this to a get_protocol_test_ignore_list function as shown below:

def get_protocol_test_ignore_list():
    ignore_list_path = os.path.join(TEST_DIR, IGNORE_LIST_FILENAME)
    with open(ignore_list_path) as f:
        return json.load(f)

Where are we setting the body to None? It looks like our fallback is empty bytes. Is this what's causing issues with the xml test above?

Is this code even reachable? How did this work before?

At the top of this file, we provide guidance for running a specific test suite:

To run a single test suite you can set the BOTOCORE_TEST_ID env var:

BOTOCORE_TEST=tests/unit/protocols/input/json.json BOTOCORE_TEST_ID=5
pytest tests/unit/test_protocols.py

However, if you follow this example you'll receive the following error:

TypeError: Invalid format for BOTOCORE_TEST_ID, should be suite_id[:test_id], and both values should be integers.

The else condition here is intended to handle this case by but instead fails due to the incorrect syntax.

The rust SDK has experienced issues (aws-sdk-rust#1095) when parsing responses from services like quicksight who populate additional union members with null values. Although the quicksight case doesn't specifically affect boto3 due to differences in modeled shape between Smithy and C2J, it’s possible for any other service to send us this type of response for a union structure.

Related Protocol Test Id: AwsJson10DeserializeAllowNulls

When parsing union structure members, botocore will use the raw member name from the HTTP response to determine if the member is know or not. If unknown, the union is populated with the following member:

“SDK_UNKNOWN_MEMBER”: {
    “name”: “<unknown member name>”
}

Botocore fails to consider union members modeled with a locationName which is used to provide an alias for a member’s name.

Solution: When determining if a union member is know, we should consider serialized names for members modeled with the locationName trait.

Realted Protocol Test Ids: PostUnionWithJsonNameResponse1, PostUnionWithJsonNameResponse2